-
Notifications
You must be signed in to change notification settings - Fork 149
Risk Trajectory Split 2 : Interpolation Strategies #1198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
One consideration could be to put this in @chahank what do you think? |
I think this is a good idea! I would then if it is not too much work rename |
| return np.exp(interpolated_log_arr) | ||
|
|
||
|
|
||
| class InterpolationStrategyBase(ABC): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know renaming can be some work, but I would clarify here that this only about interpolating impact matrices. Something like
ImpactInterpolationsBase
| def test_linear_interp_arrays2D(self): | ||
| arr_start = np.array([[10, 100], [10, 100]]) | ||
| arr_end = np.array([[20, 200], [20, 200]]) | ||
| expected = np.array([[10.0, 100.0], [20, 200]]) | ||
| result = linear_interp_arrays(arr_start, arr_end) | ||
| np.testing.assert_allclose(result, expected, rtol=self.rtol, atol=self.atol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to have a test where actual interpolation happens, and not just selection of the end points. So, one example at least with 3 rows.
| self.assertEqual(strategy.vulnerability_interp, linear_interp_arrays) | ||
|
|
||
| # Test hazard interpolation | ||
| expected_hazard_interp = linear_interp_arrays( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having mulitple different things tested in a row in one method makes it always a bit hard to work with when tests fail, as the log does not make it always super clear which part broke and why. It would be beneficial to have separate tests or maybe use @pytest.mark.parametrize
|
I think the code in itself is fine, but it is very very hard to understand what the class and methods are actually doing. I think this is because the word interpolation is used for many different type of operations. In my understanding, the "array" interpolation is supposed to represent Bayesian mixing. The "mat" inteprolation on the other hand is an interpolation between two known points. In addition, it is quite hard to comprehend what the class |
|
And thanks for splitting into separate PRs! That helps a ton. |
Co-authored-by: Chahan M. Kropf <chahan.kropf@usys.ethz.ch>
…-project/climada_python into feature/interpolation-strategies
| methods (e.g., 'Linear', 'Exponential') describing how impact outputs | ||
| should evolve between two points in time. | ||
| Impacts result from three dimensions—Exposure, Hazard, and Vulnerability— | ||
| each of which may change differently over time. Consequently, a distinct | ||
| interpolation strategy is defined for each dimension. | ||
| Exposure interpolation differs from Hazard and Vulnerability interpolation. | ||
| Changes in exposure do not alter the shape of the impact matrices, which | ||
| allows direct interpolation of the matrices themselves. For the Exposure | ||
| dimension, interpolation therefore consists of generating intermediate | ||
| impact matrices between the two time points, with exposure evolving while | ||
| hazard and vulnerability remain fixed (to either the first or second point). | ||
| In contrast, changes in Hazard may alter the | ||
| set of events between the two time points, making direct interpolation of | ||
| impact matrices impossible. Instead, impacts are first aggregated over the | ||
| event dimension (i.e. the EAI metric). The evolution of impacts is then | ||
| interpolated as a convex combination of metric sequences computed from two | ||
| scenarios: one with hazard fixed at the initial time point and one with | ||
| hazard fixed at the final time point. | ||
| The same aggregation-based interpolation approach is applied to the | ||
| Vulnerability dimension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much clearer thanks!
| @@ -0,0 +1,352 @@ | |||
| """ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will there be an integration test with real exposure / hazard / vulnerability data in the final total new module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define real ? DEMO_H5 is "real data"
Integration tests are indeed all in Split 5 (no reason to have integration test for partial part of the whole module in my sense)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm here there is no use of DEMO_H5 right? Or am I blind? Otherwise I agree, it was just to check that this was then tested. I think it would be good to have one specific integration for the interpolation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes sorry! Mixing up the split.
This one is really for the interface for strategies.
There should be unit tests with DEMO in Split 5, as well as integration tests with both simple mock data "easy" to recompute and DEMO data. I'm not sure I'm entirely finished with the integration tests though.
I'm also shifting everything to pytest format as per Lukas recommendation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!
As the Risk trajectory PR is too substantial, this is a second split.
This PR implements the concept of interpolation strategies to journey along the Risk Cube.
It:
A strategy is a triplet of callable:
exposure_interp: "how do I generate a range of given number of dates impact matrices, between present date and future date, accounting for changing exposure but constant hazard and vulnerability?"hazard_interp: "how do I interpolate between two arrays of risk metrics, the first one corresponding to present hazard and evolving exposure, and the second one to future hazard and evolving exposure, to incorporate the evolution due to the changing hazard"vulnerability_interp: same ashazard_interpbut for vulnerability.PR Author Checklist
develop)PR Reviewer Checklist